DNR: Put static and dynamic rules in the same priority space
Rulesets no longer have priorities. CompositeMatcher now returns the
highest priority rule from all its rulesets, instead of stopping after
the first ruleset.
Bug: 1026733
Change-Id: Ie77c4d1171bce374af4999339da7e73c018730e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2026747
Commit-Queue: Theodore Dubois <tbodt@google.com>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Auto-Submit: Theodore Dubois <tbodt@google.com>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741200}
diff --git a/chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc b/chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc
index c3f6d487..5cc5706 100644
--- a/chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc
+++ b/chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc
@@ -1016,9 +1016,9 @@
base::Optional<std::string> redirect_url;
} rules_data[] = {
{"google.com", 1, 1, "redirect", static_redirect_url.spec()},
- {"num=1|", 2, 2, "allow", base::nullopt},
- {"1|", 3, 1, "redirect", dynamic_redirect_url.spec()},
- {"num=21|", 4, 2, "allow", base::nullopt},
+ {"num=1|", 2, 3, "allow", base::nullopt},
+ {"1|", 3, 4, "redirect", dynamic_redirect_url.spec()},
+ {"num=3|", 4, 2, "allow", base::nullopt},
};
std::vector<TestRule> rules;
@@ -1057,6 +1057,7 @@
} static_test_cases[] = {
{get_url(0), static_redirect_url},
{get_url(1), get_url(1)},
+ {get_url(3), static_redirect_url},
};
for (const auto& test_case : static_test_cases) {
@@ -1070,18 +1071,18 @@
EXPECT_EQ(test_case.expected_final_url, final_url);
}
- // Now add dynamic rules. These should override static rules in priority.
+ // Now add dynamic rules. These should share the priority space with static
+ // rules.
const ExtensionId& extension_id = last_loaded_extension_id();
ASSERT_NO_FATAL_FAILURE(AddDynamicRules(extension_id, dynamic_rules));
- // Test that rules follow the priority of: dynamic allow > dynamic redirect >
- // static allow > static redirect.
+ // Test that dynamic and static rules are in the same priority space.
struct {
GURL initial_url;
GURL expected_final_url;
} dynamic_test_cases[] = {
{get_url(1), dynamic_redirect_url},
- {get_url(21), get_url(21)},
+ {get_url(3), get_url(3)},
};
for (const auto& test_case : dynamic_test_cases) {
diff --git a/extensions/browser/api/declarative_net_request/composite_matcher.cc b/extensions/browser/api/declarative_net_request/composite_matcher.cc
index d25dccf..c88f47d 100644
--- a/extensions/browser/api/declarative_net_request/composite_matcher.cc
+++ b/extensions/browser/api/declarative_net_request/composite_matcher.cc
@@ -35,17 +35,6 @@
return true;
}
-bool AreSortedPrioritiesUnique(const CompositeMatcher::MatcherList& matchers) {
- base::Optional<size_t> previous_priority;
- for (const auto& matcher : matchers) {
- if (matcher->priority() == previous_priority)
- return false;
- previous_priority = matcher->priority();
- }
-
- return true;
-}
-
// Helper to log the time taken in CompositeMatcher::GetBeforeRequestAction.
class ScopedGetBeforeRequestActionTimer {
public:
@@ -76,7 +65,6 @@
CompositeMatcher::CompositeMatcher(MatcherList matchers)
: matchers_(std::move(matchers)) {
- SortMatchersByPriority();
DCHECK(AreIDsUnique(matchers_));
}
@@ -94,10 +82,8 @@
if (it == matchers_.end()) {
matchers_.push_back(std::move(new_matcher));
- SortMatchersByPriority();
} else {
- // Update the matcher. The priority for a given ID should remain the same.
- DCHECK_EQ(new_matcher->priority(), (*it)->priority());
+ // Update the matcher.
*it = std::move(new_matcher);
}
@@ -113,6 +99,7 @@
ScopedGetBeforeRequestActionTimer timer;
bool notify_request_withheld = false;
+ base::Optional<RequestAction> final_action;
for (const auto& matcher : matchers_) {
base::Optional<RequestAction> action =
matcher->GetBeforeRequestAction(params);
@@ -131,10 +118,12 @@
}
}
- if (action)
- return ActionInfo(std::move(action), false);
+ final_action =
+ GetMaxPriorityAction(std::move(final_action), std::move(action));
}
+ if (final_action)
+ return ActionInfo(std::move(final_action), false);
return ActionInfo(base::nullopt, notify_request_withheld);
}
@@ -189,14 +178,5 @@
return false;
}
-void CompositeMatcher::SortMatchersByPriority() {
- std::sort(matchers_.begin(), matchers_.end(),
- [](const std::unique_ptr<RulesetMatcher>& a,
- const std::unique_ptr<RulesetMatcher>& b) {
- return a->priority() > b->priority();
- });
- DCHECK(AreSortedPrioritiesUnique(matchers_));
-}
-
} // namespace declarative_net_request
} // namespace extensions
diff --git a/extensions/browser/api/declarative_net_request/composite_matcher.h b/extensions/browser/api/declarative_net_request/composite_matcher.h
index 3133d82..0100497 100644
--- a/extensions/browser/api/declarative_net_request/composite_matcher.h
+++ b/extensions/browser/api/declarative_net_request/composite_matcher.h
@@ -80,10 +80,6 @@
private:
bool ComputeHasAnyExtraHeadersMatcher() const;
- // Sorts |matchers_| in descending order of priority.
- void SortMatchersByPriority();
-
- // Sorted by priority in descending order.
MatcherList matchers_;
// Denotes the cached return value for |HasAnyExtraHeadersMatcher|. Care must
diff --git a/extensions/browser/api/declarative_net_request/composite_matcher_unittest.cc b/extensions/browser/api/declarative_net_request/composite_matcher_unittest.cc
index 5cd9a1b..1bad580 100644
--- a/extensions/browser/api/declarative_net_request/composite_matcher_unittest.cc
+++ b/extensions/browser/api/declarative_net_request/composite_matcher_unittest.cc
@@ -45,243 +45,62 @@
DISALLOW_COPY_AND_ASSIGN(CompositeMatcherTest);
};
-// Ensure CompositeMatcher respects priority of individual rulesets.
-TEST_F(CompositeMatcherTest, RulesetPriority) {
- TestRule block_rule = CreateGenericRule();
- block_rule.condition->url_filter = std::string("google.com");
- block_rule.id = kMinValidID;
-
- TestRule redirect_rule_1 = CreateGenericRule();
- redirect_rule_1.condition->url_filter = std::string("example.com");
- redirect_rule_1.priority = kMinValidPriority;
- redirect_rule_1.action->type = std::string("redirect");
- redirect_rule_1.action->redirect.emplace();
- redirect_rule_1.action->redirect->url = std::string("http://ruleset1.com");
- redirect_rule_1.id = kMinValidID + 1;
-
- // Create the first ruleset matcher. It blocks google.com and redirects
- // example.com to ruleset1.com.
- const size_t kSource1ID = 1;
- const size_t kSource1Priority = 1;
- std::unique_ptr<RulesetMatcher> matcher_1;
- ASSERT_TRUE(CreateVerifiedMatcher(
- {block_rule, redirect_rule_1},
- CreateTemporarySource(kSource1ID, kSource1Priority), &matcher_1));
-
- // Now create a second ruleset matcher. It allows google.com and redirects
- // example.com to ruleset2.com.
- const size_t kSource2ID = 2;
- const size_t kSource2Priority = 2;
- TestRule allow_rule = block_rule;
+// Ensure that the rules in a CompositeMatcher are in the same priority space.
+TEST_F(CompositeMatcherTest, SamePrioritySpace) {
+ // Create the first ruleset matcher. It allows requests to google.com.
+ TestRule allow_rule = CreateGenericRule();
+ allow_rule.id = kMinValidID;
+ allow_rule.condition->url_filter = std::string("google.com");
allow_rule.action->type = std::string("allow");
- TestRule redirect_rule_2 = redirect_rule_1;
- redirect_rule_2.action->redirect.emplace();
- redirect_rule_2.action->redirect->url = std::string("http://ruleset2.com");
- std::unique_ptr<RulesetMatcher> matcher_2;
+ allow_rule.priority = 1;
+ std::unique_ptr<RulesetMatcher> allow_matcher;
ASSERT_TRUE(CreateVerifiedMatcher(
- {allow_rule, redirect_rule_2},
- CreateTemporarySource(kSource2ID, kSource2Priority), &matcher_2));
+ {allow_rule}, CreateTemporarySource(/*id*/ 1), &allow_matcher));
- // Create a composite matcher with the two rulesets.
+ // Now create the second matcher. It blocks requests to google.com, with
+ // higher priority than the allow rule.
+ TestRule block_rule = allow_rule;
+ block_rule.action->type = std::string("block");
+ block_rule.priority = 2;
+ std::unique_ptr<RulesetMatcher> block_matcher;
+ ASSERT_TRUE(CreateVerifiedMatcher(
+ {block_rule}, CreateTemporarySource(/*id*/ 2), &block_matcher));
+
+ // Create a composite matcher with both rulesets.
std::vector<std::unique_ptr<RulesetMatcher>> matchers;
- matchers.push_back(std::move(matcher_1));
- matchers.push_back(std::move(matcher_2));
+ matchers.push_back(std::move(allow_matcher));
+ matchers.push_back(std::move(block_matcher));
auto composite_matcher =
std::make_unique<CompositeMatcher>(std::move(matchers));
- GURL google_url = GURL("http://google.com");
- RequestParams google_params;
- google_params.url = &google_url;
- google_params.element_type = url_pattern_index::flat::ElementType_SUBDOCUMENT;
- google_params.is_third_party = false;
+ GURL google_url("http://google.com");
+ RequestParams params;
+ params.url = &google_url;
- // The second ruleset should get more priority.
- ActionInfo action_info = composite_matcher->GetBeforeRequestAction(
- google_params, PageAccess::kAllowed);
+ // The block rule should be higher priority.
+ ActionInfo action_info =
+ composite_matcher->GetBeforeRequestAction(params, PageAccess::kAllowed);
ASSERT_TRUE(action_info.action);
- EXPECT_EQ(action_info.action->type, RequestAction::Type::ALLOW);
+ EXPECT_EQ(action_info.action->type, RequestAction::Type::BLOCK);
- GURL example_url = GURL("http://example.com");
- RequestParams example_params;
- example_params.url = &example_url;
- example_params.element_type =
- url_pattern_index::flat::ElementType_SUBDOCUMENT;
- example_params.is_third_party = false;
-
- action_info = composite_matcher->GetBeforeRequestAction(example_params,
- PageAccess::kAllowed);
- ASSERT_TRUE(action_info.action);
- EXPECT_EQ(action_info.action->type, RequestAction::Type::REDIRECT);
- EXPECT_EQ(GURL("http://ruleset2.com"), action_info.action->redirect_url);
- EXPECT_FALSE(action_info.notify_request_withheld);
-
- // Now switch the priority of the two rulesets. This requires re-constructing
- // the two ruleset matchers.
- matcher_1.reset();
- matcher_2.reset();
+ // Now swap the priority of the rules, which requires re-creating the ruleset
+ // matchers and composite matcher.
+ allow_rule.priority = 2;
+ block_rule.priority = 1;
+ ASSERT_TRUE(CreateVerifiedMatcher(
+ {allow_rule}, CreateTemporarySource(/*id*/ 1), &allow_matcher));
+ ASSERT_TRUE(CreateVerifiedMatcher(
+ {block_rule}, CreateTemporarySource(/*id*/ 2), &block_matcher));
matchers.clear();
- ASSERT_TRUE(CreateVerifiedMatcher(
- {block_rule, redirect_rule_1},
- CreateTemporarySource(kSource1ID, kSource2Priority), &matcher_1));
- ASSERT_TRUE(CreateVerifiedMatcher(
- {allow_rule, redirect_rule_2},
- CreateTemporarySource(kSource2ID, kSource1Priority), &matcher_2));
- matchers.push_back(std::move(matcher_1));
- matchers.push_back(std::move(matcher_2));
+ matchers.push_back(std::move(allow_matcher));
+ matchers.push_back(std::move(block_matcher));
composite_matcher = std::make_unique<CompositeMatcher>(std::move(matchers));
- // Reusing request params means that their allow_rule_caches must be cleared.
- google_params.allow_rule_cache.clear();
- example_params.allow_rule_cache.clear();
-
- // The first ruleset should get more priority.
- action_info = composite_matcher->GetBeforeRequestAction(google_params,
- PageAccess::kAllowed);
- ASSERT_TRUE(action_info.action);
- EXPECT_TRUE(action_info.action->IsBlockOrCollapse());
-
- action_info = composite_matcher->GetBeforeRequestAction(example_params,
- PageAccess::kAllowed);
- ASSERT_TRUE(action_info.action);
- EXPECT_EQ(action_info.action->type, RequestAction::Type::REDIRECT);
- EXPECT_EQ(GURL("http://ruleset1.com"), action_info.action->redirect_url);
- EXPECT_FALSE(action_info.notify_request_withheld);
-}
-
-// Ensure allow rules in a higher priority matcher override redirect
-// and removeHeader rules from lower priority matchers.
-TEST_F(CompositeMatcherTest, AllowRuleOverrides) {
- TestRule allow_rule_1 = CreateGenericRule();
- allow_rule_1.id = kMinValidID;
- allow_rule_1.condition->url_filter = std::string("google.com");
- allow_rule_1.action->type = std::string("allow");
-
- TestRule remove_headers_rule_1 = CreateGenericRule();
- remove_headers_rule_1.id = kMinValidID + 1;
- remove_headers_rule_1.condition->url_filter = std::string("example.com");
- remove_headers_rule_1.action->type = std::string("removeHeaders");
- remove_headers_rule_1.action->remove_headers_list =
- std::vector<std::string>({"referer", "setCookie"});
-
- // Create the first ruleset matcher, which allows requests to google.com and
- // removes headers from requests to example.com.
- const size_t kSource1ID = 1;
- const size_t kSource1Priority = 1;
- std::unique_ptr<RulesetMatcher> matcher_1;
- ASSERT_TRUE(CreateVerifiedMatcher(
- {allow_rule_1, remove_headers_rule_1},
- CreateTemporarySource(kSource1ID, kSource1Priority,
- dnr_api::SOURCE_TYPE_MANIFEST),
- &matcher_1));
-
- // Now set up rules and the second matcher.
- TestRule allow_rule_2 = allow_rule_1;
- allow_rule_2.condition->url_filter = std::string("example.com");
-
- TestRule redirect_rule_2 = CreateGenericRule();
- redirect_rule_2.condition->url_filter = std::string("google.com");
- redirect_rule_2.priority = kMinValidPriority;
- redirect_rule_2.action->type = std::string("redirect");
- redirect_rule_2.action->redirect.emplace();
- redirect_rule_2.action->redirect->url = std::string("http://ruleset2.com");
- redirect_rule_2.id = kMinValidID + 1;
-
- // Create a second ruleset matcher, which allows requests to example.com and
- // redirects requests to google.com.
- const size_t kSource2ID = 2;
- const size_t kSource2Priority = 2;
- std::unique_ptr<RulesetMatcher> matcher_2;
- ASSERT_TRUE(
- CreateVerifiedMatcher({allow_rule_2, redirect_rule_2},
- CreateTemporarySource(kSource2ID, kSource2Priority,
- dnr_api::SOURCE_TYPE_DYNAMIC),
- &matcher_2));
-
- // Create a composite matcher with the two rulesets.
- std::vector<std::unique_ptr<RulesetMatcher>> matchers;
- matchers.push_back(std::move(matcher_1));
- matchers.push_back(std::move(matcher_2));
- auto composite_matcher =
- std::make_unique<CompositeMatcher>(std::move(matchers));
-
- // Send a request to google.com which should be redirected.
- GURL google_url = GURL("http://google.com");
- RequestParams google_params;
- google_params.url = &google_url;
- google_params.element_type = url_pattern_index::flat::ElementType_SUBDOCUMENT;
- google_params.is_third_party = false;
-
- // The second ruleset should get more priority.
- ActionInfo action_info = composite_matcher->GetBeforeRequestAction(
- google_params, PageAccess::kAllowed);
- ASSERT_TRUE(action_info.action);
- EXPECT_EQ(action_info.action->type, RequestAction::Type::REDIRECT);
- EXPECT_EQ(GURL("http://ruleset2.com"), action_info.action->redirect_url);
- EXPECT_FALSE(action_info.notify_request_withheld);
-
- // Send a request to example.com with headers, expect the allow rule to be
- // matched and the headers to remain.
- GURL example_url = GURL("http://example.com");
- RequestParams example_params;
- example_params.url = &example_url;
- example_params.element_type =
- url_pattern_index::flat::ElementType_SUBDOCUMENT;
- example_params.is_third_party = false;
-
- // Expect no headers to be removed.
- std::vector<RequestAction> remove_header_actions;
- EXPECT_EQ(0u, composite_matcher->GetRemoveHeadersMask(
- example_params, 0u, &remove_header_actions));
- EXPECT_TRUE(remove_header_actions.empty());
-
- remove_header_actions.clear();
-
- // Now switch the priority of the two rulesets. This requires re-constructing
- // the two ruleset matchers.
- matcher_1.reset();
- matcher_2.reset();
- matchers.clear();
- ASSERT_TRUE(
- CreateVerifiedMatcher({allow_rule_1, remove_headers_rule_1},
- CreateTemporarySource(kSource1ID, kSource2Priority,
- dnr_api::SOURCE_TYPE_DYNAMIC),
- &matcher_1));
- ASSERT_TRUE(CreateVerifiedMatcher(
- {allow_rule_2, redirect_rule_2},
- CreateTemporarySource(kSource2ID, kSource1Priority,
- dnr_api::SOURCE_TYPE_MANIFEST),
- &matcher_2));
- matchers.push_back(std::move(matcher_1));
- matchers.push_back(std::move(matcher_2));
- composite_matcher = std::make_unique<CompositeMatcher>(std::move(matchers));
-
- // Reusing request params means that their allow_rule_caches must be cleared.
- google_params.allow_rule_cache.clear();
- example_params.allow_rule_cache.clear();
-
- // The first ruleset should get more priority and so the request to google.com
- // should not be redirected.
- action_info = composite_matcher->GetBeforeRequestAction(google_params,
- PageAccess::kAllowed);
+ // The allow rule should now have higher priority.
+ action_info =
+ composite_matcher->GetBeforeRequestAction(params, PageAccess::kAllowed);
ASSERT_TRUE(action_info.action);
EXPECT_EQ(action_info.action->type, RequestAction::Type::ALLOW);
- EXPECT_FALSE(action_info.notify_request_withheld);
-
- // The request to example.com should now have its headers removed.
- example_params.allow_rule_cache.clear();
- uint8_t expected_mask =
- flat::RemoveHeaderType_referer | flat::RemoveHeaderType_set_cookie;
- EXPECT_EQ(expected_mask, composite_matcher->GetRemoveHeadersMask(
- example_params, 0u, &remove_header_actions));
- ASSERT_EQ(1u, remove_header_actions.size());
-
- RequestAction expected_action = CreateRequestActionForTesting(
- RequestAction::Type::REMOVE_HEADERS, *remove_headers_rule_1.id,
- kDefaultPriority, dnr_api::SOURCE_TYPE_DYNAMIC);
- expected_action.request_headers_to_remove.push_back(
- net::HttpRequestHeaders::kReferer);
- expected_action.response_headers_to_remove.push_back("set-cookie");
- EXPECT_EQ(expected_action, remove_header_actions[0]);
}
// Tests that header masks are correctly attributed to rules for multiple
@@ -300,38 +119,31 @@
};
TestRule static_rule_1 = create_remove_headers_rule(
- kMinValidID, "g*", std::vector<std::string>({"referer", "cookie"}));
-
- TestRule static_rule_2 = create_remove_headers_rule(
- kMinValidID + 1, "g*", std::vector<std::string>({"setCookie"}));
+ kMinValidID, "google.com", std::vector<std::string>({"cookie"}));
TestRule dynamic_rule_1 = create_remove_headers_rule(
- kMinValidID, "google.com", std::vector<std::string>({"referer"}));
+ kMinValidID, "/path", std::vector<std::string>({"referer"}));
TestRule dynamic_rule_2 = create_remove_headers_rule(
- kMinValidID + 2, "google.com", std::vector<std::string>({"setCookie"}));
+ kMinValidID + 1, "/path", std::vector<std::string>({"setCookie"}));
// Create the first ruleset matcher, which matches all requests with "g" in
// their URL.
const size_t kSource1ID = 1;
- const size_t kSource1Priority = 1;
std::unique_ptr<RulesetMatcher> matcher_1;
ASSERT_TRUE(CreateVerifiedMatcher(
- {static_rule_1, static_rule_2},
- CreateTemporarySource(kSource1ID, kSource1Priority,
- dnr_api::SOURCE_TYPE_MANIFEST),
+ {static_rule_1},
+ CreateTemporarySource(kSource1ID, dnr_api::SOURCE_TYPE_MANIFEST),
&matcher_1));
// Create a second ruleset matcher, which matches all requests from
// |google.com|.
const size_t kSource2ID = 2;
- const size_t kSource2Priority = 2;
std::unique_ptr<RulesetMatcher> matcher_2;
- ASSERT_TRUE(
- CreateVerifiedMatcher({dynamic_rule_1, dynamic_rule_2},
- CreateTemporarySource(kSource2ID, kSource2Priority,
- dnr_api::SOURCE_TYPE_DYNAMIC),
- &matcher_2));
+ ASSERT_TRUE(CreateVerifiedMatcher(
+ {dynamic_rule_1, dynamic_rule_2},
+ CreateTemporarySource(kSource2ID, dnr_api::SOURCE_TYPE_DYNAMIC),
+ &matcher_2));
// Create a composite matcher with the two rulesets.
std::vector<std::unique_ptr<RulesetMatcher>> matchers;
@@ -340,7 +152,7 @@
auto composite_matcher =
std::make_unique<CompositeMatcher>(std::move(matchers));
- GURL google_url = GURL("http://google.com");
+ GURL google_url = GURL("http://google.com/path");
RequestParams google_params;
google_params.url = &google_url;
google_params.element_type = url_pattern_index::flat::ElementType_SUBDOCUMENT;
@@ -378,33 +190,6 @@
::testing::Eq(::testing::ByRef(static_action_1)),
::testing::Eq(::testing::ByRef(dynamic_action_1)),
::testing::Eq(::testing::ByRef(dynamic_action_2))));
-
- GURL gmail_url = GURL("http://gmail.com");
- RequestParams gmail_params;
- gmail_params.url = &gmail_url;
- gmail_params.element_type = url_pattern_index::flat::ElementType_SUBDOCUMENT;
- gmail_params.is_third_party = false;
-
- actions.clear();
- EXPECT_EQ(expected_mask, composite_matcher->GetRemoveHeadersMask(
- gmail_params, 0u, &actions));
-
- static_action_1 = CreateRequestActionForTesting(
- RequestAction::Type::REMOVE_HEADERS, *static_rule_1.id,
- dnr_api::SOURCE_TYPE_MANIFEST);
- static_action_1.request_headers_to_remove.push_back(
- net::HttpRequestHeaders::kCookie);
- static_action_1.request_headers_to_remove.push_back(
- net::HttpRequestHeaders::kReferer);
-
- RequestAction static_action_2 = CreateRequestActionForTesting(
- RequestAction::Type::REMOVE_HEADERS, *static_rule_2.id, kDefaultPriority,
- dnr_api::SOURCE_TYPE_MANIFEST);
- static_action_2.response_headers_to_remove.push_back("set-cookie");
-
- EXPECT_THAT(actions, ::testing::UnorderedElementsAre(
- ::testing::Eq(::testing::ByRef(static_action_1)),
- ::testing::Eq(::testing::ByRef(static_action_2))));
}
// Ensure CompositeMatcher detects requests to be notified based on the rule
@@ -424,12 +209,9 @@
upgrade_rule.action->type = std::string("upgradeScheme");
upgrade_rule.id = kMinValidID + 1;
- const size_t kSource1ID = 1;
- const size_t kSource1Priority = 1;
std::unique_ptr<RulesetMatcher> matcher_1;
- ASSERT_TRUE(CreateVerifiedMatcher(
- {redirect_rule, upgrade_rule},
- CreateTemporarySource(kSource1ID, kSource1Priority), &matcher_1));
+ ASSERT_TRUE(CreateVerifiedMatcher({redirect_rule, upgrade_rule},
+ CreateTemporarySource(), &matcher_1));
// Create a composite matcher.
std::vector<std::unique_ptr<RulesetMatcher>> matchers;
@@ -520,12 +302,9 @@
// In terms of priority: ghi > def > abc.
- const size_t kSource1ID = 1;
- const size_t kSource1Priority = 1;
std::unique_ptr<RulesetMatcher> matcher_1;
- ASSERT_TRUE(CreateVerifiedMatcher(
- {abc_redirect, def_upgrade, ghi_redirect},
- CreateTemporarySource(kSource1ID, kSource1Priority), &matcher_1));
+ ASSERT_TRUE(CreateVerifiedMatcher({abc_redirect, def_upgrade, ghi_redirect},
+ CreateTemporarySource(), &matcher_1));
// Create a composite matcher.
std::vector<std::unique_ptr<RulesetMatcher>> matchers;
diff --git a/extensions/browser/api/declarative_net_request/file_sequence_helper.cc b/extensions/browser/api/declarative_net_request/file_sequence_helper.cc
index b4e340f..8056c1c1 100644
--- a/extensions/browser/api/declarative_net_request/file_sequence_helper.cc
+++ b/extensions/browser/api/declarative_net_request/file_sequence_helper.cc
@@ -224,9 +224,9 @@
// Initially write the new JSON and indexed rulesets to temporary files to
// ensure we don't leave the actual files in an inconsistent state.
std::unique_ptr<RulesetSource> temporary_source =
- RulesetSource::CreateTemporarySource(
- source.id(), source.priority(), source.type(),
- source.rule_count_limit(), source.extension_id());
+ RulesetSource::CreateTemporarySource(source.id(), source.type(),
+ source.rule_count_limit(),
+ source.extension_id());
if (!temporary_source) {
*error = kInternalErrorUpdatingDynamicRules;
*status = UpdateDynamicRulesStatus::kErrorCreateTemporarySource;
diff --git a/extensions/browser/api/declarative_net_request/request_action.cc b/extensions/browser/api/declarative_net_request/request_action.cc
index 71399d7..f716c86e7 100644
--- a/extensions/browser/api/declarative_net_request/request_action.cc
+++ b/extensions/browser/api/declarative_net_request/request_action.cc
@@ -36,8 +36,8 @@
return rhs;
if (!rhs)
return lhs;
- return lhs->index_priority > rhs->index_priority ? std::move(lhs)
- : std::move(rhs);
+ return lhs->index_priority >= rhs->index_priority ? std::move(lhs)
+ : std::move(rhs);
}
} // namespace declarative_net_request
diff --git a/extensions/browser/api/declarative_net_request/ruleset_matcher.cc b/extensions/browser/api/declarative_net_request/ruleset_matcher.cc
index 66169766..f176ba7 100644
--- a/extensions/browser/api/declarative_net_request/ruleset_matcher.cc
+++ b/extensions/browser/api/declarative_net_request/ruleset_matcher.cc
@@ -55,9 +55,9 @@
// Using WrapUnique instead of make_unique since this class has a private
// constructor.
- *matcher = base::WrapUnique(new RulesetMatcher(
- std::move(ruleset_data), source.id(), source.priority(), source.type(),
- source.extension_id()));
+ *matcher = base::WrapUnique(new RulesetMatcher(std::move(ruleset_data),
+ source.id(), source.type(),
+ source.extension_id()));
return kLoadSuccess;
}
@@ -112,13 +112,11 @@
RulesetMatcher::RulesetMatcher(
std::string ruleset_data,
size_t id,
- size_t priority,
api::declarative_net_request::SourceType source_type,
const ExtensionId& extension_id)
: ruleset_data_(std::move(ruleset_data)),
root_(flat::GetExtensionIndexedRuleset(ruleset_data_.data())),
id_(id),
- priority_(priority),
url_pattern_index_matcher_(extension_id,
source_type,
root_->index_list(),
diff --git a/extensions/browser/api/declarative_net_request/ruleset_matcher.h b/extensions/browser/api/declarative_net_request/ruleset_matcher.h
index 75a3e37..2eeee96 100644
--- a/extensions/browser/api/declarative_net_request/ruleset_matcher.h
+++ b/extensions/browser/api/declarative_net_request/ruleset_matcher.h
@@ -86,10 +86,6 @@
// their own unique ids.
size_t id() const { return id_; }
- // Priority of the ruleset. Each extension can have multiple rulesets with
- // their own different priorities.
- size_t priority() const { return priority_; }
-
// Returns the tracked highest priority matching allowsAllRequests action, if
// any, for |host|.
base::Optional<RequestAction> GetAllowlistedFrameActionForTesting(
@@ -98,7 +94,6 @@
private:
explicit RulesetMatcher(std::string ruleset_data,
size_t id,
- size_t priority,
api::declarative_net_request::SourceType source_type,
const ExtensionId& extension_id);
@@ -107,7 +102,6 @@
const flat::ExtensionIndexedRuleset* const root_;
const size_t id_;
- const size_t priority_;
// Underlying matcher for filter-list style rules supported using the
// |url_pattern_index| component.
diff --git a/extensions/browser/api/declarative_net_request/ruleset_matcher_unittest.cc b/extensions/browser/api/declarative_net_request/ruleset_matcher_unittest.cc
index bef87d5..bb0b7b0 100644
--- a/extensions/browser/api/declarative_net_request/ruleset_matcher_unittest.cc
+++ b/extensions/browser/api/declarative_net_request/ruleset_matcher_unittest.cc
@@ -321,11 +321,10 @@
std::unique_ptr<RulesetMatcher> matcher;
const size_t kId = 1;
- const size_t kPriority = 1;
const size_t kRuleCountLimit = 10;
ASSERT_TRUE(CreateVerifiedMatcher(
{rule},
- CreateTemporarySource(kId, kPriority,
+ CreateTemporarySource(kId,
api::declarative_net_request::SOURCE_TYPE_MANIFEST,
kRuleCountLimit),
&matcher));
diff --git a/extensions/browser/api/declarative_net_request/ruleset_source.cc b/extensions/browser/api/declarative_net_request/ruleset_source.cc
index fa174e64..9a6f3d0 100644
--- a/extensions/browser/api/declarative_net_request/ruleset_source.cc
+++ b/extensions/browser/api/declarative_net_request/ruleset_source.cc
@@ -47,10 +47,6 @@
namespace dnr_api = extensions::api::declarative_net_request;
using Status = ReadJSONRulesResult::Status;
-// Dynamic rulesets get more priority over static rulesets.
-const size_t kStaticRulesetPriority = 1;
-const size_t kDynamicRulesetPriority = 2;
-
constexpr const char kFileDoesNotExistError[] = "File does not exist.";
constexpr const char kFileReadError[] = "File read error.";
@@ -237,8 +233,8 @@
return RulesetSource(
declarative_net_request::DNRManifestData::GetRulesetPath(extension),
file_util::GetIndexedRulesetPath(extension.path()), kStaticRulesetID,
- kStaticRulesetPriority, dnr_api::SOURCE_TYPE_MANIFEST,
- dnr_api::MAX_NUMBER_OF_RULES, extension.id());
+ dnr_api::SOURCE_TYPE_MANIFEST, dnr_api::MAX_NUMBER_OF_RULES,
+ extension.id());
}
// static
@@ -251,14 +247,13 @@
return RulesetSource(
dynamic_ruleset_directory.AppendASCII(kDynamicRulesJSONFilename),
dynamic_ruleset_directory.AppendASCII(kDynamicIndexedRulesFilename),
- kDynamicRulesetID, kDynamicRulesetPriority, dnr_api::SOURCE_TYPE_DYNAMIC,
+ kDynamicRulesetID, dnr_api::SOURCE_TYPE_DYNAMIC,
dnr_api::MAX_NUMBER_OF_DYNAMIC_RULES, extension.id());
}
// static
std::unique_ptr<RulesetSource> RulesetSource::CreateTemporarySource(
size_t id,
- size_t priority,
dnr_api::SourceType type,
size_t rule_count_limit,
ExtensionId extension_id) {
@@ -271,20 +266,18 @@
return std::make_unique<RulesetSource>(
std::move(temporary_file_json), std::move(temporary_file_indexed), id,
- priority, type, rule_count_limit, std::move(extension_id));
+ type, rule_count_limit, std::move(extension_id));
}
RulesetSource::RulesetSource(base::FilePath json_path,
base::FilePath indexed_path,
size_t id,
- size_t priority,
dnr_api::SourceType type,
size_t rule_count_limit,
ExtensionId extension_id)
: json_path_(std::move(json_path)),
indexed_path_(std::move(indexed_path)),
id_(id),
- priority_(priority),
type_(type),
rule_count_limit_(rule_count_limit),
extension_id_(std::move(extension_id)) {}
@@ -294,8 +287,8 @@
RulesetSource& RulesetSource::operator=(RulesetSource&&) = default;
RulesetSource RulesetSource::Clone() const {
- return RulesetSource(json_path_, indexed_path_, id_, priority_, type_,
- rule_count_limit_, extension_id_);
+ return RulesetSource(json_path_, indexed_path_, id_, type_, rule_count_limit_,
+ extension_id_);
}
IndexAndPersistJSONRulesetResult
diff --git a/extensions/browser/api/declarative_net_request/ruleset_source.h b/extensions/browser/api/declarative_net_request/ruleset_source.h
index f330583..4e496a5 100644
--- a/extensions/browser/api/declarative_net_request/ruleset_source.h
+++ b/extensions/browser/api/declarative_net_request/ruleset_source.h
@@ -133,7 +133,6 @@
// Returns null on failure.
static std::unique_ptr<RulesetSource> CreateTemporarySource(
size_t id,
- size_t priority,
api::declarative_net_request::SourceType type,
size_t rule_count_limit,
ExtensionId extension_id);
@@ -141,7 +140,6 @@
RulesetSource(base::FilePath json_path,
base::FilePath indexed_path,
size_t id,
- size_t priority,
api::declarative_net_request::SourceType type,
size_t rule_count_limit,
ExtensionId extension_id);
@@ -157,9 +155,8 @@
// Path to the indexed flatbuffer rules.
const base::FilePath& indexed_path() const { return indexed_path_; }
- // Each ruleset source within an extension has a distinct ID and priority.
+ // Each ruleset source within an extension has a distinct ID.
size_t id() const { return id_; }
- size_t priority() const { return priority_; }
// The origin type for this ruleset. Can be from the manifest or dynamic.
api::declarative_net_request::SourceType type() const { return type_; }
@@ -205,7 +202,6 @@
base::FilePath json_path_;
base::FilePath indexed_path_;
size_t id_;
- size_t priority_;
api::declarative_net_request::SourceType type_;
size_t rule_count_limit_;
ExtensionId extension_id_;
diff --git a/extensions/browser/api/declarative_net_request/test_utils.cc b/extensions/browser/api/declarative_net_request/test_utils.cc
index 40144881..729bf10 100644
--- a/extensions/browser/api/declarative_net_request/test_utils.cc
+++ b/extensions/browser/api/declarative_net_request/test_utils.cc
@@ -281,12 +281,11 @@
}
RulesetSource CreateTemporarySource(size_t id,
- size_t priority,
dnr_api::SourceType source_type,
size_t rule_count_limit,
ExtensionId extension_id) {
std::unique_ptr<RulesetSource> source = RulesetSource::CreateTemporarySource(
- id, priority, source_type, rule_count_limit, std::move(extension_id));
+ id, source_type, rule_count_limit, std::move(extension_id));
CHECK(source);
return source->Clone();
}
diff --git a/extensions/browser/api/declarative_net_request/test_utils.h b/extensions/browser/api/declarative_net_request/test_utils.h
index 301e28b..fa3d7340 100644
--- a/extensions/browser/api/declarative_net_request/test_utils.h
+++ b/extensions/browser/api/declarative_net_request/test_utils.h
@@ -68,7 +68,6 @@
// Helper to return a RulesetSource bound to temporary files.
RulesetSource CreateTemporarySource(
size_t id = 1,
- size_t priority = 1,
api::declarative_net_request::SourceType source_type =
api::declarative_net_request::SOURCE_TYPE_MANIFEST,
size_t rule_count_limit = 100,
